io/ompio: change the default value of mca parameter #2424
Conversation
change the default value of the mca_io_ompio_cycle_buffer_size parameter in order to avoid accidental truncation of a file for very large individual operations. Thanks to @cniethammer for reporting it. Signed-off-by: Edgar Gabriel <egabriel@central.uh.edu>
|
@cniethammer would you mind reviewing this change, since you reported the problem and successfully ran the application after applying this change? |
There was a problem hiding this comment.
This prevents the issue in first place.
But the logic for the automatic buffer size selection (to be max_data) in ompi/mca/io/ompio/io_ompio_file_write.c and ompi/mca/io/ompio/io_ompio_file_read.c should be either improved to something like
bytes_per_cycle = (max_data < WRITE_SIZE_LIMIT) ?max_data : WRITE_SIZE_LIMIT;
where WRITE_SIZE_LIMIT should be platform dependent (on Linux 2,147,479,552)
or removed as it still checks for the value -1 and can trigger the issue when the user wants to use "auto" mode.
|
That's a good point, but I would ask maybe @jsquyres to chime in on that: to the best of knowledge, the Open MPI philosophy with MCA parameters was always to follow the request of the user, even if this leads to an error. In this particular situation, you are absolutely right that if the user would disable the cycle_buffer_size feature by setting it to -1, his code might fail. However, after this merge, this would only happen if he explicitly requested that behavior. Your suggestion would basically overwrite the request of the user. A second more practical difficulty is, that there is to the best of my knowledge no predefined constant in header files that we could just use. There is RLIMIT_FSIZE which indicates the maximum file size overall, the posix write manpages talks about SSIZE_MAX, both of which are however way too large. So this would require developing the entire configure logic etc. to make it portable. I can do that if the community decides that this is the way to go, but in that case this fix might come too late for the 2.0.2 release. |
|
We talked about this in email. The decision was:
In short: more work still needs to be done -- this is a short-term fix while this more detailed work is ongoing. |
jsquyres
dismissed
cniethammer’s stale review
Per #2424 (comment), allowing this PR to go through.
change the default value of the mca_io_ompio_cycle_buffer_size parameter in order to avoid accidental truncation of a file for very large individual operations.
Thanks to @cniethammer for reporting it.
Signed-off-by: Edgar Gabriel egabriel@central.uh.edu